-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove inconsistent top-level document helpers #161
Remove inconsistent top-level document helpers #161
Conversation
I kinda agree here. Ugh. Can we add factories w/ implementations to extension types |
Yep! Or any other static method. That would be preferable to this as they would be scoped to that type and could be added more consistently. I think there's other potential designs to consider as well. |
That's WAY more discoverable!!! |
Hmm, this is a tough call. I do think these members are a better way to instantiate elements as it avoids the need for a string, but I recognize that they pollute the namespace. While we can put them into the extension types, that'll require some manual intervention in the generator scripts. If the methods to create them aren't consistent (for example, On the other hand, we can't really put them into extensions as:
|
I don't disagree there, but as a developer, I would ask why do I create a
I'd argue that it probably shouldn't. In the current naming standards and setup of this package, it's relying heavily on being consistent with the experience in JS. So users familiar with JS or those reading docs like MDN provide can be applicable for uses here. I think deprecating these now is separate from what would be best, as it gives us time and room to determine an API that we like and is applicable to more types. If it ends up being like these, adding them back will be much easier than taking them away. As another example, I'll add that my preferred method of creating elements would be something like: void main() {
final canvas = document.newCreateElement<HTMLCanvasElement>();
}
extension on Document {
T newCreateElement<T extends HTMLElement>([JSAny? options]) =>
document.createElement(magicTypeToName[T]!) as T;
}
// This would be replaced with some compiler magic or something:
const magicTypeToName = {
HTMLCanvasElement: 'canvas',
// ... Continued
}; |
Adding every single one was the original plan. :)
I'd agree with you. We're in a weird stage where we're trying to bridge the gap between I do agree we should expose these better. I think until we have the ability to actually declare these as factories in an extension, the compromise might be to include these in the generator as additional factories. It goes against the principle of separating out the helpers and the generated code, but it's probably too useful to not do this.
I think this is a bit too magical, and I'm not sure every element has a predictable naming scheme here (maybe they do!). |
I think these helpers are more harmful to the developer experience than helpful as they pollute the global scope and are inconsistent with every other usage of
package:web
. It was more acceptable when they were guarded behind thehelpers.dart
library, but now there's no way to avoid these without explicitly hiding them.